Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create App for Scope using fullscreen #76

Open
wants to merge 7 commits into
base: production
Choose a base branch
from

Conversation

pablolins
Copy link

@pablolins pablolins commented Oct 5, 2020

First of all thank you for creating this amazing piece of software.

This pull request adds an App instead of an Applet for the Scope functionality.
I've created in the past a feature request for it #40,
but now can finally try to contribute.

The main motivation behind it is to use the entire screen.

how the appp works

  • Only 1 channel is being used
  • CLOCK input 1 will sync the ticks to the bpm and display the bpm in the app header
  • CV input 1 will be drawn in scope area and the voltage will shown in the app header
  • CV out 1 will be the same as CV input 1
  • Left encoder will change the number of ticks as in the applet. Currently the number of ticks isnt being displayed anywhere but I guess if needed It could be sequeezed at bottom.

Since the screen is already small as I user I wont use it for more than one channel.

Things to consider:

  • Maybe there isnt the wish to adding this new App because to do so I guess something will get deleted, but this PR doesnt delete anything
  • If its decided that change is welcome then I would happily adapt the implementation to better fit the project :)
  • Also, I would gladly update the WIKI if necessarty
  • The code is basically copied from the scope applet, maybe theres a more efficient way of doing it, like abstracting things into another method IDK

Copy link

@no-trbl-2-u no-trbl-2-u left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phenomenal job here! No blocking comments, just a few nitpicks. Keep up the great work!

(This is definitely an applet I wanted to see! Honestly, I thought if I just had the scope applet active on both sides, the left scope would merge with the right scope and the left CV1 would just forward the information to the right scope and they'd be able to display full screen. Obviously that's not the case, so I'm glad we've got someone working on this 💯 )

Comment on lines +77 to +91
void OnLeftButtonLongPress() {

}

void OnRightButtonPress() {
}

void OnUpButtonPress() {
}

void OnDownButtonPress() {
}

void OnDownButtonLongPress() {
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these methods have to be defined in order to prevent null references down the line? Or is it just common practice in this repo to define all the possible event handlers?

Comment on lines +125 to +127
// void DrawTicks() {
// gfxPrint(40, 1, sample_ticks);
// }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Please remove any commented out code or uncomment this if it's needed.
If this has to stay, please leave a comment on why it's commented out and when it should be uncommented.

Comment on lines +131 to +141
int v = (cv * 100) / (12 << 7);
bool neg = v < 0 ? 1 : 0;
if (v < 0) v = -v;
int wv = v / 100; // whole volts
int dv = v - (wv * 100); // decimal
gfxPrint(neg ? "-" : "+");
gfxPrint(wv);
gfxPrint(".");
if (dv < 10) gfxPrint("0");
gfxPrint(dv);
gfxPrint("V");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Could you get some formatting done in here? Obviously it's of zero importance in the grand scheme of things, but readability is king 👑

Comment on lines +198 to +217
void BigScope_handleButtonEvent(const UI::Event &event) {
// For left encoder, handle press and long press
if (event.control == OC::CONTROL_BUTTON_L) {
if (event.type == UI::EVENT_BUTTON_LONG_PRESS) BigScope_instance.OnLeftButtonLongPress();
else BigScope_instance.OnLeftButtonPress();
}

// For right encoder, only handle press (long press is reserved)
if (event.control == OC::CONTROL_BUTTON_R && event.type == UI::EVENT_BUTTON_PRESS)
BigScope_instance.OnRightButtonPress();

// For up button, handle only press (long press is reserved)
if (event.control == OC::CONTROL_BUTTON_UP) BigScope_instance.OnUpButtonPress();

// For down button, handle press and long press
if (event.control == OC::CONTROL_BUTTON_DOWN) {
if (event.type == UI::EVENT_BUTTON_PRESS) BigScope_instance.OnDownButtonPress();
if (event.type == UI::EVENT_BUTTON_LONG_PRESS) BigScope_instance.OnDownButtonLongPress();
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pristine 🛀 Great Job!

suit4 added a commit to suit4/O_C-HemisphereSuite that referenced this pull request Jun 17, 2022
Adding the the BigScope App
suit4 added a commit to suit4/O_C-HemisphereSuite that referenced this pull request Jun 17, 2022
Adding the the BigScope App
suit4 added a commit to suit4/O_C-HemisphereSuite that referenced this pull request Jun 17, 2022
Adding the BigScope App
suit4 added a commit to suit4/O_C-HemisphereSuite that referenced this pull request Jun 17, 2022
Adding BigScope App – Changes from pull request Chysn#76
Changes from pull request Chysn#51
Changes from pull request Chysn#105
Changes from pull request Logarhythm1#2
suit4 added a commit to suit4/O_C-HemisphereSuite that referenced this pull request Jun 17, 2022
Adding BigScope App – Changes from pull request Chysn#76
Changes from pull request Chysn#51
Changes from pull request Chysn#105
Changes from pull request Logarhythm1#2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants